Support For Arbitrary Number of Keychains #318

thunderbiscuit · 2025-09-24T18:10:08Z

Description

This PR allows the Wallet type to track any number of keychains (descriptors). It is a breaking change. Because of the size of the diff, we'll try to make it reviewable 1 commit at a time. See "Reviewing the Diff" section for a breakdown of each commit.

Related Issues: #188, #227
Related PRs: #230, #226
Exploratory Crate: multi-keychain-wallet

Read this one-pager for an overview of user-facing impacts of the proposed feature.

Todo

Reviewing the Diff

The diff on this PR is fairly big because it touches a lot of the wallet, even if the changes are not that "deep" in terms of business logic.

In order to make it easy for reviewers, we'll be trying to keep the commit history easy to review one commit at a time. Once the PR is ready to merge, we can squash some of those commits in order to simplify the commit history if need be.

The following is a table that outlines what each commit does.

Commit	Notes
1	Add the new `KeyRing` type.
2	Add a few tests for this new KeyRing.
3	Add the `ChangeSet` required for the `KeyRing.
4	Comment out tests and examples that are wallet-dependent. This will make further commits that bring back modified tests easier to review individually. At this point source and examples compile, and tests run (of course because about 80% are commented out!).
5	Modified the wallet to hold a KeyRing internally. Modified the ChangeSet accordingly.
6	Add new constructor `Wallet::new` which takes a single argument, a `KeyRing`.
7	Modify `AddressInfo` to take in a generic keychain. Add `Wallet::reveal_next_address`, `Wallet::reveal_next_default_address` APIs.
8	Add secondary constructor, `Wallet::with_custom_params`.
9	Add the simplest example of wallet creation. 2-descriptors, external + internal
10	Add a more complex example of a wallet that tracks many keychains.

Changelog notice

TODO

Deployment Strategy

As per discussions over different dev calls, this feature is proposed as a breaking change on the Wallet API, since the old constructors are removed. It might not be ready in time for the 3.0 release (given the size of the diff and its relative position on the totem pole of priorities). If it does not make the cut for the 3.0 release, it will simply be kept updated and rebased on master until it is ready for merging into a breaking change ready branch for the 4.0 release.

coveralls · 2025-09-24T18:15:16Z

Pull Request Test Coverage Report for Build 18313801420

Details

89 of 118 (75.42%) changed or added relevant lines in 2 files are covered.
456 unchanged lines in 8 files lost coverage.
Overall coverage decreased (-17.8%) to 67.449%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/wallet/changeset.rs	9	10	90.0%
src/wallet/mod.rs	80	108	74.07%

Files with Coverage Reduction	New Missed Lines	%
src/wallet/tx_builder.rs	3	87.38%
src/wallet/utils.rs	6	91.3%
src/wallet/error.rs	8	0.0%
src/wallet/changeset.rs	18	44.44%
src/wallet/export.rs	19	41.18%
src/wallet/mod.rs	24	42.29%
src/wallet/params.rs	119	0.0%
src/wallet/signer.rs	259	29.62%

Totals
Change from base Build 18099939060:	-17.8%
Covered Lines:	1695
Relevant Lines:	2513

💛 - Coveralls

110CodingP · 2025-09-25T19:36:11Z

The next few commits:

modify the Wallet struct and add a basic constructor
add some address apis
add another constructor for advanced users to provide things like a custom genesis_hash.

thunderbiscuit

Quick review on 6c50f00: given your commit message I thought you were adding the Wallet::new constructor here, but the commit only contains the addition of the KeyRing to the Wallet type. Mind you this might be enough for this commit. Feel free to add a new commit with the constructor, or add it to this commit; whatever you think works best.

Also note that fa984e6 and 6c50f00 don't compile because of

impl AsRef<bdk_chain::tx_graph::TxGraph<ConfirmationBlockTime>> for Wallet {
    fn as_ref(&self) -> &bdk_chain::tx_graph::TxGraph<ConfirmationBlockTime> {
        self.keychain_tx_graph.graph()
    }
}

on line 2638 of wallet/mod.rs. You could comment it out with the rest to ensure everything builds.

thunderbiscuit · 2025-09-29T00:16:04Z

wallet/src/wallet/changeset.rs

+    pub keyring: keyring::changeset::ChangeSet<K>,
    /// Changes to the [`LocalChain`](local_chain::LocalChain).
-    pub local_chain: local_chain::ChangeSet,
+    pub chain: local_chain::ChangeSet,


I'm not sure if the renaming of this field is intentional and I don't have an opinion on whether it's a good idea or not yet, but it probably belongs in a different PR.

I did the rename since the corresponding field in Wallet is so. reverted the rename in 588121b.

110CodingP · 2025-09-29T05:18:17Z

I am so sorry 😢 . I completely messed dividing the commits into two. Things should be fixed now!

thunderbiscuit · 2025-09-30T17:27:38Z

This one now needs a big ol' rebase @110CodingP.

added #![allow(unused)] to circumvent clippy errors.

Modified the `ChangeSet` accordingly.

Also added default version of `reveal_next_address`.

So that advanced users can specify custom genesis_hash and lookahead or enable spk_cache.

110CodingP · 2025-10-01T14:03:04Z

Wow! this one was HUGE! For the first few commits I did something like git rebase --onto master HEAD~7 HEAD~6 and for the later ones I cherry-picked each commit onto master and finally did a reset --hard, wondering if there's a simpler way to do difficult rebases 😅 ?

thunderbiscuit · 2025-10-03T17:45:47Z

Adding an example like this fails:

    // Simple KeyRing, allowing us to build a standard 2-descriptor wallet.
    let external_descriptor: &str = "tr(tpubD6NzVbkrYhZ4WyC5VZLuSJQ14uwfUbus7oAFurAFkZA5N3groeQqtW65m8pG1TT1arPpfWu9RbBsc5rSBncrX2d84BAwJJHQfaRjnMCQwuT/86h/1h/0h/0/*)";
    let internal_descriptor: &str = "tr(tpubD6NzVbkrYhZ4WyC5VZLuSJQ14uwfUbus7oAFurAFkZA5N3groeQqtW65m8pG1TT1arPpfWu9RbBsc5rSBncrX2d84BAwJJHQfaRjnMCQwuT/86h/1h/0h/1/*)";

    let mut keyring: KeyRing<KeychainKind> = KeyRing::new(Network::Regtest, KeychainKind::External, external_descriptor);
    keyring.add_descriptor(KeychainKind::Internal, internal_descriptor, false);

    let mut wallet = Wallet::new(keyring);

With a stacktrace containing:

thread 'main' panicked at /Users/user/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/miniscript-12.3.5/src/descriptor/key.rs:687:14:
The key should not contain any wildcards at this point

After a bit of poking around I found that this is coming from the descriptor_id() method called inside insert_descriptor():

    pub fn insert_descriptor(
        &mut self,
        keychain: K,
        descriptor: Descriptor<DescriptorPublicKey>,
    ) -> Result<bool, InsertDescriptorError<K>> {
        let did = descriptor.descriptor_id();

I don't have time to finish the investigation today, but just pointing it out. I'm not sure yet why this worked well in the multi-keychain-wallet crate and not here, nor why the code for the constructor is so different than the other previous constructors like Wallet::create_with_params.

110CodingP · 2025-10-04T06:03:30Z

This almost scared me for a moment 😅 ! Using descriptors with unhardened paths seems to work.
This got me into thinking about the other constructor with_custom_params , currently the genesis_hash takes precedence over keyring's network so should there be a check in the constructor which asserts that if KeyRing has Mainnet as the network the genesis_hash should also be of the Mainnet? and similarly for testnets and regtest?

thunderbiscuit · 2025-10-06T12:35:17Z

Total facepalm. Sorry @110CodingP! Ok so I simplified the example a little bit just to keep a really neat version of how to build a wallet as close as you can from the 2.0 approach.

Also added a folder to consolidate example utilities.

thunderbiscuit requested a review from ValuedMammal as a code owner September 24, 2025 18:10

github-project-automation bot added this to BDK Wallet Sep 24, 2025

thunderbiscuit force-pushed the feature/multi-keychain-wallet branch from 7059451 to 0df1a2a Compare September 25, 2025 18:45

110CodingP force-pushed the feature/multi-keychain-wallet branch from 524d4df to 6c50f00 Compare September 28, 2025 17:49

thunderbiscuit commented Sep 29, 2025

View reviewed changes

110CodingP force-pushed the feature/multi-keychain-wallet branch 2 times, most recently from 7c976e1 to 588121b Compare September 29, 2025 05:15

thunderbiscuit and others added 8 commits October 1, 2025 18:32

feat: add KeyRing type

e4d5b6a

test: add simple KeyRing tests

2694bfb

feat: add KeyRing changeset

e5a824d

refactor!: comment out Wallet dependant code.

237ecdf

added #![allow(unused)] to circumvent clippy errors.

feat!: modify Wallet struct to hold a KeyRing

2af1d58

Modified the `ChangeSet` accordingly.

feat: Add a basic constructor for Wallet

8d94a22

feat!: modify AddressInfo, add reveal_next_address

01dc8ff

Also added default version of `reveal_next_address`.

feat: add Wallet::with_custom_params API

4a3486e

So that advanced users can specify custom genesis_hash and lookahead or enable spk_cache.

110CodingP force-pushed the feature/multi-keychain-wallet branch from dfdcd83 to 4a3486e Compare October 1, 2025 13:54

thunderbiscuit force-pushed the feature/multi-keychain-wallet branch 3 times, most recently from 23d0e7a to 20096ff Compare October 6, 2025 12:41

docs: add KeychainKind based example

29fd25c

Also added a folder to consolidate example utilities.

thunderbiscuit force-pushed the feature/multi-keychain-wallet branch 2 times, most recently from 9134ce2 to ac4c44f Compare October 7, 2025 13:03

thunderbiscuit force-pushed the feature/multi-keychain-wallet branch 3 times, most recently from 042f5e2 to 4403cdb Compare October 7, 2025 13:07

docs: add complex multi-keychain example

16d6f8c

thunderbiscuit force-pushed the feature/multi-keychain-wallet branch from 4403cdb to 16d6f8c Compare October 7, 2025 13:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Support For Arbitrary Number of Keychains #318

Support For Arbitrary Number of Keychains #318

Uh oh!

thunderbiscuit commented Sep 24, 2025 •

edited

Loading

Uh oh!

coveralls commented Sep 24, 2025 •

edited

Loading

Uh oh!

110CodingP commented Sep 25, 2025

Uh oh!

thunderbiscuit left a comment •

edited

Loading

Uh oh!

thunderbiscuit Sep 29, 2025

Uh oh!

110CodingP Sep 29, 2025

Uh oh!

110CodingP commented Sep 29, 2025

Uh oh!

thunderbiscuit commented Sep 30, 2025

Uh oh!

110CodingP commented Oct 1, 2025

Uh oh!

thunderbiscuit commented Oct 3, 2025

Uh oh!

110CodingP commented Oct 4, 2025

Uh oh!

thunderbiscuit commented Oct 6, 2025

Uh oh!

Uh oh!

Support For Arbitrary Number of Keychains #318

Are you sure you want to change the base?

Support For Arbitrary Number of Keychains #318

Uh oh!

Conversation

thunderbiscuit commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Todo

Reviewing the Diff

Changelog notice

Deployment Strategy

Uh oh!

coveralls commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 18313801420

Details

💛 - Coveralls

Uh oh!

110CodingP commented Sep 25, 2025

Uh oh!

thunderbiscuit left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thunderbiscuit Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

110CodingP Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

110CodingP commented Sep 29, 2025

Uh oh!

thunderbiscuit commented Sep 30, 2025

Uh oh!

110CodingP commented Oct 1, 2025

Uh oh!

thunderbiscuit commented Oct 3, 2025

Uh oh!

110CodingP commented Oct 4, 2025

Uh oh!

thunderbiscuit commented Oct 6, 2025

Uh oh!

Uh oh!

thunderbiscuit commented Sep 24, 2025 •

edited

Loading

coveralls commented Sep 24, 2025 •

edited

Loading

thunderbiscuit left a comment •

edited

Loading